Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve UX for "Close mode" settings in editor preference page #2298

Conversation

praveen-skp
Copy link
Contributor

Issue: #2297

This pull request adds a "Close mode" frame in the editor preference to make the "Close editors automatically" and "Number of opened editors before closing" grouping clear.

@praveen-skp
Copy link
Contributor Author

Close mode

The new UI with "Close mode" grouping

@laeubi
Copy link
Contributor

laeubi commented Sep 19, 2024

Close mode as a group name reads a bit strange here, I would expect then having different modes to select but the configuration do not really offer that.

My suggestion would be to merge both settings into one line instead of grouping them e.g.

[ ] Close oldest if there are more than _____ editors open

Copy link
Contributor

github-actions bot commented Sep 19, 2024

Test Results

 1 818 files  + 1   1 818 suites  +1   1h 33m 19s ⏱️ - 6m 3s
 7 709 tests ± 0   7 481 ✅ + 1  228 💤 ± 0  0 ❌  - 1 
24 288 runs  +45  23 541 ✅ +36  747 💤 +10  0 ❌  - 1 

Results for commit 263f51c. ± Comparison against base commit a022116.

♻️ This comment has been updated with latest results.

@BeckerWdf
Copy link
Contributor

Close mode as a group name reads a bit strange here

What about "Closing behaviour"?

@BeckerWdf
Copy link
Contributor

My suggestion would be to merge both settings into one line instead of grouping them

This is also a good idea.

@praveen-skp
Copy link
Contributor Author

This is how the new UI looks.
Disabled
Enabled

@BeckerWdf
Copy link
Contributor

Can the input field made shorter so that the line doesn't get too long. I think place for around 4 chars would be sufficient.

What about changing the text to: "Close oldest editor if there more then ... editor open"

What does "oldest" mean?
a) The one that was open the longest time, or
b) the one where the time of last use is most far in the past?

@praveen-skp
Copy link
Contributor Author

I checked the current behaviour and noticed that it's not always the oldest editors that are being closed.

Use case 1:

If I have the following editors opened in order:
1, 2, 3, 4, 5, 6 (1 being the oldest, focus is on 6)

  • Set editor limit to 3
  • Open editor 7
  • The current available editors are 7, 2, 3

It deletes the latest 3 editors (6, 5, 4) and replaces the oldest editor.


Use case 2:

If I have the following editors opened in order:
1, 2, 3, 4, 5, 6 (1 being the oldest, focus is on 5)

  • Set editor limit to 3
  • Open editor 7
  • The current available editors are 7, 2, 3

It deletes the latest 3 editors (5, 6, 4) and replaces the oldest editor (1).


Use case 3:

If I have the following editors opened in order:
1, 2, 3, 4, 5, 6 (1 being the oldest, focus is on 2)

  • Set editor limit to 3
  • Open editor 7
  • The current available editors are 7, 3, 4

It deletes the latest 3 editors (2, 6, 5) and replaces the oldest editor (1).


Use case 4:

If I have the following editors opened in order:
1, 2, 3 (1 being the oldest, focus is on 3)

  • Set editor limit to 3
  • Open editor 4
  • The current available editors are 4, 2, 3

It replaces the oldest editor (1).


Use case 5:

If I have the following editors opened in order:
1, 2, 3 (1 being the oldest, focus is on 1)

  • Set editor limit to 3
  • Open editor 4
  • The current available editors are 1, 4, 3

It replaces the oldest editor (2).

@praveen-skp
Copy link
Contributor Author

It looks like its not always the oldest editors that are being closed.
So shall we change the text to something generic like:
Close editor(s) if there are more than ... editors open

@BeckerWdf
Copy link
Contributor

Close editor(s) if there are more than ... editors open

yes.

@BeckerWdf
Copy link
Contributor

Close editor(s) if there are more than ... editors open

yes. Maybe without the () simply "Close editors if..."

@praveen-skp
Copy link
Contributor Author

Reduced size

Reduced the size of the input field to show two characters, to match the text limit of 2 characters in the code.

@BeckerWdf
Copy link
Contributor

merge commit are not allowed in PR. Can you pls. squash and rebase on top of current master please?

@praveen-skp praveen-skp force-pushed the group_close_editor_options_in_preference branch from b88650b to c24a5a4 Compare October 10, 2024 08:57
@BeckerWdf BeckerWdf force-pushed the group_close_editor_options_in_preference branch from c24a5a4 to b7aaaa7 Compare October 11, 2024 08:55
@BeckerWdf BeckerWdf changed the title Added "Close mode" frame in editor preference Improve UX for "Close mode" settings in editor preference page Oct 11, 2024
Make more clear that these two setting belong together.

Change-Id: Idf0c683df571ae124e8166e4dcf6bb9536debbb3
@BeckerWdf BeckerWdf force-pushed the group_close_editor_options_in_preference branch from b7aaaa7 to 263f51c Compare October 11, 2024 11:13
Copy link
Contributor

@BeckerWdf BeckerWdf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I plan to merge this once CI-Build is happy

@BeckerWdf BeckerWdf added this to the 4.34 M2 milestone Oct 11, 2024
@BeckerWdf BeckerWdf merged commit 090a18a into eclipse-platform:master Oct 11, 2024
17 checks passed
@praveen-skp praveen-skp deleted the group_close_editor_options_in_preference branch November 7, 2024 01:55
@praveen-skp praveen-skp restored the group_close_editor_options_in_preference branch November 7, 2024 01:55
@praveen-skp praveen-skp deleted the group_close_editor_options_in_preference branch November 7, 2024 01:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants